zend_cfg: use uint32_t type rather than int type#21542
Conversation
| int backtracking_block_num = block_num; | ||
| do { | ||
| block_num--; | ||
| } while (block_num >= 0 | ||
| backtracking_block_num--; | ||
| } while (backtracking_block_num >= 0 | ||
| && !(ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)); | ||
| if (block_num >= 0) { | ||
| if (backtracking_block_num >= 0) { | ||
| continue; |
There was a problem hiding this comment.
I feel that there should be a better way to solve this but when I initially did:
| int backtracking_block_num = block_num; | |
| do { | |
| block_num--; | |
| } while (block_num >= 0 | |
| backtracking_block_num--; | |
| } while (backtracking_block_num >= 0 | |
| && !(ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)); | |
| if (block_num >= 0) { | |
| if (backtracking_block_num >= 0) { | |
| continue; | |
| do { | |
| block_num--; | |
| } while (block_num > 0 | |
| && !(ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)); | |
| if (!(ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)) { | |
| continue; |
it didn't behave as expected, so I guess I'm not fully grasping what is happening here.
There was a problem hiding this comment.
I think that you had inverted the logic in if (!(ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)) {. Before, if (block_num >= 0) implied that ssa->cfg.blocks[block_num] was reachable. So it should be if ((ssa->cfg.blocks[block_num].flags & ZEND_BB_REACHABLE)) {.
But I find the original logic easier to follow.
There was a problem hiding this comment.
Happy to keep the original logic, although I might add a comment explaining what is happening :)
arnaud-lb
left a comment
arnaud-lb
left a comment
There was a problem hiding this comment.
Did a partial review up to dfa_pass.c
These are count values and thus should never be negative, more over their use site usually wants them to be uint32_t too. Aligning these types makes the intention clearer. I have also trying to propagate those changes to use sites of these struct members.
Girgias
force-pushed
the
blocks_count_uint32t
branch
from
9966838 to
31201ca
Compare
|
Needed to force-push after a rebase due to conflicts arising after the merge of #21540, but follow-up commits address the review comments. :) |
3 participants
These are count values and thus should never be negative, more over their use site usually wants them to be uint32_t too. Aligning these types makes the intention clearer.
I have also trying to propagate those changes to use sites of these struct members.